-
Notifications
You must be signed in to change notification settings - Fork 81
IBX-10885: Document Content Type search API #2946
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 5.0
Are you sure you want to change the base?
Conversation
Preview of modified filesPreview of modified Markdown: |
code_samples/ change report
|
| new Criterion\LogicalAnd([ | ||
| new Criterion\ContentTypeIdentifier(['folder', 'article']), | ||
| ]), | ||
| new Criterion\ContentTypeIdentifier(['user']), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would make sense to use some different criterion here -> maybe ContentTypeGroupName. The current one doesn't make a lot of sense because it could be simply put into one criterion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just relied on an example given in your PR. We can add ContentTypeGroupName in a follow up PR, WDYS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use something else than new Criterion\ContentTypeIdentifier(['user']), it can be even ContentTypeGroupId. The current example is not a proper one, these criteria:
new Criterion\LogicalOr([
new Criterion\LogicalAnd([
new Criterion\ContentTypeIdentifier(['folder', 'article']),
]),
new Criterion\ContentTypeIdentifier(['user']),shouldn't be composed like that. LogicalAnd doesn't make sense without at least two criterions inside. All this logic could be replaced with:
new Criterion\ContentTypeIdentifier(['folder', 'article', 'user'])
Let's keep this example grounded
Document Content Type search API
Checklist